Correction for the commit message. The DB and the Cuepointer do update, but the UI doesn't.

Thank you. You can correct commit messages by:

git commit --amend
git push -f

Thank you for adopting this! :)

You can correct commit messages by:
git commit --amend
git push -f

I know but I don't like to amend commits that I have already pushed (since its somewhat rewriting the history which never works out).

I know but I don't like to amend commits that I have already pushed (since its somewhat rewriting the history which never works out).

Generally, it's okay to force push for a pull request. If anyone else is working together with you on the same branch, force pushing can cause hassles. Also, when you force push GitHub can't associate old comments with the specific lines of code anymore.

Thanks for working on improving Mixxx's cue point capabilities! IMO this is the biggest remaining weak point of Mixxx compared to other DJ software. Let's take this big project step-by-step. I think a good goal for this first pull request will be integrating color selection into the track properties dialog and using that color in the waveform marks. Creating an easier to use UI for manipulating cue points could be done in another PR after this. Before you put in the work to do that, you may want to work on https://bugs.launchpad.net/mixxx/+bug/1277689 to allow easier access to the existing track properties window.

This is ready to be merged IMHO...
But I don't know why Codefactor is complaining about the public: being on a blank line. Isn't that the standard?

The CodeFactor issue is:
"public:" should be preceded by a blank line
I think they want blocks of public and private. Somhow a kind of false positive.
So you can either add a blank line above private to silence CodeFactor or ignore it.
It does IMHO not prevent this PR from merging.

Works good.
There is only an issue that the cue number can not be read with light colors.
Can you invert the font color once a certain lightness value of the cue color is passed?

The color field in the cue table can become a combo box with for example these colors:
https://sashat.me/2017/01/11/list-of-20-simple-distinct-colors/
This prevents us that beta users clutters their library with random colors.
It would also OK for me if you change the edited Hex value to the closest palette color.

There is only an issue that the cue number can not be read with light colors.
Can you invert the font color once a certain lightness value of the cue color is passed?

I have done some tests and determined that in many circumstances, the the contrast is to low in general. The compatible colors are really depended on the context/skin in which they are displayed atleast in the overview. My current solution would be color the markers in their set color with enough contrast to the signalcolors (achieved via some borders around the marker), while still having the number within the overview in the color that is chosen by the skin (instead of the marker color) for better visibility. I have changed that the waveformrendermark to switch to a white textcolor when the color gets to dark (like you have suggested).

While I'm playing with the colors and looking at the overview, I think we should reduce the palette and 'merge' similar colors:

• Blue, Cyan, Navy, Teal
• Pink, Lavender, Apricot, Beige
• Purple, Magenta
• Green, Olive, Teal
• Yellow & Lime also very close, but both nice colors

Agree, I've chosen
Red,
Green,
Yellow,
Blue,
Cyan,
Magenta,
Pink,
Teal,
Grey,

Would this work for you?

So we have "no color" for all existing cues set by the skin and override able by the user. I like that idea.

This will also help to adjust all existing cues to the schema in the users mind. We can't predict the users meaning of default red.

How about white instead of grey?

This will also help to adjust all existing cues to the schema in the users mind. We can't predict the users meaning of default red.

I'm sold with this argument :)
I'll try to implement it this way.

How about white instead of grey?

I'd avoid white because it won't work very well with a white skin. Like this one we used to have.
https://lh6.googleusercontent.com/-YATHf2XGrD0/T1YyRukZARI/AAAAAAAAAB4/q3uV4zGcLS0/s640/Mixxx1.10-Phoney.png

How about white instead of grey?

well, I'm reluctant to drop neon green in LateNight.. IMO that's an essential indicator color there

I would prefere to use rather more usable colours then less.

I would prefere to use rather more usable colours then less.

Just because we should be prepared to import competitors colours. The user can finally decide if a colour is useful.

Sorry for the close cycle. I hate this button.

Just because we should be prepared to import competitors colours. The user can finally decide if a colour is useful.

good point (although I have no idea how competitors handles this)

The problem is that we must only offer colors that can be distinguishable. The current list has colors that are too similar, and also white and black which won't render nice in all skins.

I'd avoid white because it won't work very well with a white skin.

You're right. Then white could also be used as a reserved controller color (eg. to highlight a specific hotcue) which could be useful.

Sorry for the close cycle. I hat this button.

and afaik there's no way to report that issue to github itself, except the contact form.
I use github-dark theme in ff, and there it's easy to just remove that button.

There's one issue if we change the default cue colors in the future:

1. User sets blue color to a hotcue. #4363D8 is stored in database (the default blue hex code)
2. In a future release we change the default blue color to #0000FF
3. User loads the track in the new version. Mixxx tries to find which default color has code #4363D8.
   Since no default color has such code, the hotcue gets assigned the Invalid color.

This will happen for all blue hotcues.

Ways to fix this:

1. If we change a default color, we need to add a new DB scheme version and change the color code for all affected cues.
2. Store the default color name in DB instead of the Hex code. We don't allow custom colors anyway, so why store the HEX code?

@daschuer Thoughts?

Would a relational database solve the problem?
I mean if you had a table for the cuecolors within the DB (not the C++ source) you could also allow the user to edit that table (to add more colors/change colors for example). The tracks themselves would then just store a reference to the color in the dedicated color table. Skins could then just define/populate their own colortable variants and mixxx would access the right color table depending on the current skin.
This is just how I would do it when I were to develop something in PHP so this might not be the right approach in our case.

@Swiftb0y Thanks for the suggestion. Yes that's pretty much my proposal, but without storing to the DB, just reference color objects in code.

Well I thought that the colors in the database would allow for more flexibility in the long run. I am not that set on preventing users completely from adding custom cue colors. Would it be possible to integrate that similar to how skins are able to override colors? Just with the exception that the user colors are stored in a separate table instead of an/(the skins) XML file?

Store the default color name in DB instead of the Hex code. We don't allow custom colors anyway, so why store the HEX code?

I don't necessarily want to restart the discussion again whether we should allow custom colors but IMO it would be feasible than before if we stored the colors as IDs instead of their absolute value.

Yes, in a future we might allow custom colors. But right now we are just implementing predefined colors. One step at a time :)

Store the default color name in DB instead of the Hex code. We don't allow custom colors anyway, so why store the HEX code?

I ended up implementing this solution, but storing a color id instead of name. Thus we will be able to change both the HEX code and the name of a color safely.

From the controller scripts point of view, the experience is pretty much the same. Before we needed to call a function to decode the CO value into an usable RGB color. Now you will be able to call a function to get an RGB color from the id. Same thing.

Commits coming soon

but storing a color id instead of name.

That was my Idea as well as it seems more flexible.

Commits coming soon

I'm very hyped. Thank you continuing what I started (even though I suspect that there won't be much of my code left once you're done ;))

That was my Idea as well as it seems more flexible.

Thank you for the hint ;)

I'm very hyped. Thank you continuing what I started (even though I suspect that there won't be much of my code left once you're done ;))

I've heavily refactored color.h. It was a mess (my fault, I added too much things there).
The brightness function is very useful and it's still there ;). I've changed some of the accompanying functions. You prepared everything so we didn't need to calculate the brightness of a color all the time but just once. However, we were actually calculating it all the time. I fixed that :)

I've opened #2016.
Shall we close this PR?

definitely.